NE-2032: e2e: Signal service deprovisioning issues during Gateway DNS test #1220
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
alebedev87
changed the title
openshift-ci
bot
added
the
do-not-merge/work-in-progress
alebedev87
force-pushed
the
dnsrecord-listener-update
branch
from
301e0bc to
a079a5a
Compare
|
/retest-required |
1 similar comment
|
/retest-required |
This adds a check for the service associated with a deleted gateway. The test now waits for the service to be fully removed and logs a warning if it persists beyond a specified timeout. This helps identify delays in deprovisioning of cloud load balancers backing gateway services.
alebedev87
force-pushed
the
dnsrecord-listener-update
branch
from
a079a5a to
849cdc8
Compare
alebedev87
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
alebedev87
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@alebedev87: This pull request references NE-2032 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/assign @grzpiotrowski |
|
/retest |
1 similar comment
|
/retest |
|
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
openshift-ci
bot
added
the
lifecycle/stale
|
/remove-lifecycle stale |
openshift-ci
bot
removed
the
lifecycle/stale
|
|
||
| if err := kclient.Delete(context.TODO(), gateway); err != nil { | ||
| t.Errorf("failed to delete gateway %q: %v", gateway.Name, err) | ||
| t.Fatalf("Failed to delete gateway %q: %v", gateway.Name, err) |
There was a problem hiding this comment.
do we care about the same problems of networking here? eg.: do you want to retry the delete also?
Additionally, if you do add this delete to the retry function below, it is worth ignoring the error if the object does not exist, as previous loop may have deleted it.
One more comment, out of this change but above on line 646: I would add a RetryOnConflict for that update/patch, as you may have other controllers (GatewayAPI/OSSM) changing it, the Update may fail with a conflict. It would be good to ignore and retry the update
Edit: OTOH it may lead to a false negative if you try to get a service name that doesn't match the gateway name, as it will be not found
|
|
||
| // The load balancer deprovisioning can take some time. | ||
| // Signal a long deprovisioning to help distinguish it from DNS management problems. | ||
| gtwSvcName := types.NamespacedName{Namespace: "openshift-ingress", Name: "test-gateway-update-openshift-default"} |
There was a problem hiding this comment.
IIRC the service name is derived from the Gateway name, so instead of calling it "test-gateway-update-openshift-default" do you want to compose the name from the gateway name? This way in case of some change on some logic/naming it will not break the test
openshift-merge-robot
added
the
needs-rebase
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
1 similar comment
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@alebedev87: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
7 participants
This PR adds a check for the service associated with a deleted gateway. The test now waits for the service to be fully removed and logs a warning if it persists beyond a specified timeout. This helps identify delays in deprovisioning of cloud load balancers backing gateway services.